[Core] Change all toggles in core commands to setters#4460
[Core] Change all toggles in core commands to setters#4460Dav-Git wants to merge 5 commits intoCog-Creators:V3/developfrom
Conversation
|
Do you have an interest in expanding this to other cogs, for example Mod's |
|
Yes. The goal is to slowly convert all of core to make it uniform. For the sake of simplicity I though it would be best to split into seperate PRs per cored cog. (Hence I did warnings seperately) but if you want one big PR I can definitely do that as well. |
|
So I'd like to state my official stance on this. I don't think we should restrict functionality here, we should instead allow all commands like this to behave as both toggle AND setters doing the following. @embedset.command(name="global")
@checks.is_owner()
async def embedset_global(self, ctx: commands.Context):
async def embedset_global(self, ctx: commands.Context, enabled: bool = None):
"""
Toggle the global embed setting.
Set the global embed setting.
This is used as a fallback if the user
or guild hasn't set a preference. The
default is to use embeds.
"""
if enabled is None:
enabled = not (await self.bot._config.embeds())
await self.bot._config.embeds.set(enabled)
await ctx.send(
_("Embeds are now {} by default.").format(_("disabled") if enabled else _("enabled"))
) |
|
Seems we could go for some unification here on this field with setters and toggles. When discussing other unrelated things the idea popped up of unifying setting commands, in a way that they work both with a toggle and a boolean setter. |
|
If I understand your comment correctly the thought is to have an optional bool argument that can set a value and otherwise default to a toggle? That sounds like a decent idea to me. I'd like to see more comments on this before I make changes to the PR though. Personally I am in favor. |
|
I use this strategy often in Fox cogs, but sometimes no value is "reset to default" instead of toggle when it's not a bool but a multi select. Overall I'm in favor. |
|
I'm sorry for taking this long but I completely forgot about the suggestions here. |
Type
Description of the changes
This changes all toggles in core commands to be bool setters to be consistent with
I also updated the wording in the docstrings for these commands to reflect that they are not toggles anymore.